Fix HLSLParser failing on parenthesized expressions#948
Conversation
kblaschke
left a comment
kblaschke
left a comment
There was a problem hiding this comment.
Yup, that fixes the issue!
Would be interesting how many presets were affected by this. At some point I might add a test build that loads all the default presets and counts how many fail in a specific way. Won't tell us how compatible presets are in regards to visuals, but how many we can actually load successfully.
I have added a bullet point for "test which presets load and which fail to load" to the visual regression testing RFC. So it doesn't get lost. I suspect we could get this information as a step in the larger visual regression test; and could break it out in a way that it can be used in other contexts. |
The parser rejected valid HLSL like `float2 var = (float2(x,y)) * scalar` with "expected ';'" errors. When parsing a parenthesized expression, the loop would break before consuming the closing paren, so subsequent binary operators were never seen. Moves end-char consumption into the else block and checks for operators after consuming the paren, continuing the loop if one is found. Fixes #940
Covers the fix for issue #940 with tests for: - Parenthesized constructor with binary operators - Double-nested parentheses - Both operands parenthesized - Chained operators after parens
kblaschke
force-pushed
the
fix/hlsl-parser-paren-expression
branch
from
de135c9 to
de1f1b6
Compare
Fixes parser rejecting valid HLSL like
float2 var = (float2(x,y)) * scalarwith "expected ';'" errors.When parsing a parenthesized expression,
ParseBinaryExpression()would break from its loop before consuming the closing paren. The subsequent binary operator was never seen because the return statement consumed the)and exited immediately.The fix moves end-char consumption into the else block and checks for a following binary operator after consuming the paren, continuing the loop if one is found.
Credit to @rootnotez for the detailed analysis and test cases that identified the root cause.
Fixes #940